Skip to content

BG-834: Recurrent cascade#200

Open
ndiezel0 wants to merge 6 commits intomasterfrom
BG-834/customers
Open

BG-834: Recurrent cascade#200
ndiezel0 wants to merge 6 commits intomasterfrom
BG-834/customers

Conversation

@ndiezel0
Copy link
Contributor

No description provided.

@ndiezel0 ndiezel0 requested a review from WWWcool March 19, 2026 21:59
Copy link
Contributor

@WWWcool WWWcool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не понял в какой момент происходит добавление реккурентного токена к карте/кастумеру - это по идее на получение от провайдера должно работать, но кода нет. Тестов мало, нужен тесты на каскад и подтверждением работы

CustomerID = <<"test-customer-42">>,
RecurrentParent = ?recurrent_parent(Invoice1ID, Payment1ID),
BaseParams = make_recurrent_payment_params(true, RecurrentParent, ?pmt_sys(<<"visa-ref">>)),
Payment2Params = BaseParams#payproc_InvoicePaymentParams{customer_id = CustomerID},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если не передать парент реккурент - сохранится?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил тест. Сохранится.

{genlib, {git, "https://github.com/valitydev/genlib.git", {tag, "v1.1.0"}}},
{woody, {git, "https://github.com/valitydev/woody_erlang.git", {tag, "v1.1.1"}}},
{damsel, {git, "https://github.com/valitydev/damsel.git", {tag, "v2.2.27"}}},
{damsel, {git, "https://github.com/valitydev/damsel.git", {branch, "BG-834/customer_payment"}}},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не забыть

Comment on lines +24 to +25
{ok, Customer} = call(customer_management, 'Create', {#customer_CustomerParams{party_ref = PartyConfigRef}}),
Customer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ключа идемпотентности нет, в параметрах создания нет привязки, значит если что то пойдет не так - у тебя будет миллиард пустых кастумеров

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Например создание и сразу привязка к платежу? А если платеж не успешен? Я думал чтобы список платежей в кастомере был списком успешных платежей

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это все верно, но после создания может же быть проблема - а можно делать чтобы у тебя в случае если не было кастумера и нужно создать ты мог в рамках транзакции создать, добавить карту и добавить реккурент - при этом проверяя не было ли уже их - чтобы не было дублей?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Карты и токены добавляются идемпотентно, в этом плане проблем нет

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Идемпотентностью создания самого кастомера (это будет происходить в капи) займется бендер на основе ExternalID

Comment on lines +466 to +473
#customer_RecurrentToken{
id = <<"parent">>,
provider_ref = ProviderRef,
terminal_ref = TerminalRef,
token = RecToken,
created_at = CreatedAt,
status = {active, #customer_RecurrentTokenActive{}}
}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

звучит странно - ты его делаешь, но в кубасити его нет, кто то кро прочитает события пойдет спрашивать что то по ним, а там ничего

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

то есть сама идея что нужно учесть родительский вроде ок

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправлено

payment = #domain_InvoicePayment{
id = PaymentID,
make_recurrent = true,
customer_id = CustomerID,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если кастумера не было мб создать?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это будет происходить на уровне капи

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

но у тебя же есть проверка - when CustomerID =/= undefined - то есть ты предполагаешь что такой вариант возможен - а я как раз на него и пишу - что не создать ли в таком случае?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это sanity check проверка. Могут быть сценарии в который мы не хотим ассоциировать платеж с кастомером, потому он не передается. Например этот функционал кому-то конкретно не нужен. Тогда для него создаваться не будет. Но пусть это решает бордер, а не сам ХГ.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

так это вопрос тогда к логике - кто управляет созданием - только фронт дает сигнал или мы сами создаем - у меня было ощущение что нам нужно агрегаты эти заводить в любом случае, а если сверху кто то спросил - условно есть ли у этого реккурента ассоциированный кастумер - отдать готового

T;
error ->
%% Cascade route without pre-existing token — use parent's token
get_recurrent_token(get_payment_state(InvoiceID, PaymentID))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если это токен другого провайдера?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убрал этот кейс. Его не может быть.

Comment on lines +24 to +25
{ok, Customer} = call(customer_management, 'Create', {#customer_CustomerParams{party_ref = PartyConfigRef}}),
Customer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это все верно, но после создания может же быть проблема - а можно делать чтобы у тебя в случае если не было кастумера и нужно создать ты мог в рамках транзакции создать, добавить карту и добавить реккурент - при этом проверяя не было ли уже их - чтобы не было дублей?

CID =/= undefined
->
case hg_customer_client:get_recurrent_tokens(InvID, PmtID) of
[_ | _] = Tokens -> [?cascade_tokens_loaded(Tokens)];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а есть же length для длинны массива

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same same

ParentToken = make_parent_recurrent_token(ParentSt),
AllTokens = [ParentToken | CubastyTokens],
[?cascade_tokens_loaded(AllTokens)];
case {InheritedCustomerID, PayerParams} of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

что то метание вышло - то есть если у нас есть реккурент родительский, но его еще нет в кубасити, то мы его никак не обрабатываем? а почему прогрев базы не делать таким образом?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потому что тогда нам надо прогревать еще и всех детей родителя (сколько их?). Я не уверен что этим стоит заниматься посреди проведения платежа

case (get_payment(ParentPayment))#domain_InvoicePayment.customer_id of
CustomerID -> CustomerID;
undefined -> CustomerID;
_Other -> throw(#payproc_InvalidRecurrentParentPayment{details = <<"Customer ID mismatch with parent">>})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я так и не понял пока нафиг этот кастумер передается сверху, если ты его можешь определять по родительскому платежу?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Чем отличается родительский платеж от неродительского? И там и там мы можем передать CustomerID, InvoicePaymentParams один и тот же.

payment = #domain_InvoicePayment{
id = PaymentID,
make_recurrent = true,
customer_id = CustomerID,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

но у тебя же есть проверка - when CustomerID =/= undefined - то есть ты предполагаешь что такой вариант возможен - а я как раз на него и пишу - что не создать ли в таком случае?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants